Skip to content

Fix SubscribeAsync silently failing when Dapr sidecar is unavailable#1744

Open
GeorgeTsiokos wants to merge 3 commits intodapr:masterfrom
GeorgeTsiokos:fix/subscribe-async-silent-failure-1663
Open

Fix SubscribeAsync silently failing when Dapr sidecar is unavailable#1744
GeorgeTsiokos wants to merge 3 commits intodapr:masterfrom
GeorgeTsiokos:fix/subscribe-async-silent-failure-1663

Conversation

@GeorgeTsiokos
Copy link

@GeorgeTsiokos GeorgeTsiokos commented Mar 13, 2026

Summary

Fixes #1663

  • Throw on initial connection failure: SubscribeAsync now catches RpcException from the gRPC stream setup and throws a DaprException with context (topic name, pubsub name), allowing callers to detect and handle sidecar unavailability
  • Error callback for runtime errors: Added SubscriptionErrorHandler delegate and ErrorHandler property on DaprSubscriptionOptions so callers can receive DaprException-wrapped errors from background tasks instead of having them become UnobservedTaskExceptions
  • Retryable after failure: Resets hasInitialized flag on both initial connection failures and mid-stream background task failures, so callers can retry SubscribeAsync
  • Safe error handler invocation: Guards ErrorHandler?.Invoke() with try/catch to prevent a faulty handler from recreating unobserved exceptions
  • ContinueWith uses CancellationToken.None: Ensures error-handling continuations always execute even after cancellation

Test plan

  • HandleTaskCompletion invokes ErrorHandler with DaprException wrapping the original exception
  • HandleTaskCompletion does not throw when no ErrorHandler is set
  • HandleTaskCompletion does not throw when ErrorHandler itself throws
  • HandleTaskCompletion does not invoke ErrorHandler for successful tasks
  • SubscribeAsync throws DaprException wrapping RpcException when sidecar unavailable
  • SubscribeAsync allows retry after RpcException failure (hasInitialized reset)
  • SubscribeAsync resets hasInitialized for non-RPC exceptions (e.g. ObjectDisposedException)
  • All 22 tests pass on net8.0, net9.0, and net10.0
  • Tests use xUnit v3 patterns (TestContext.Current.CancellationToken)

🤖 Generated with Claude Code

@GeorgeTsiokos GeorgeTsiokos requested review from a team as code owners March 13, 2026 10:19
@GeorgeTsiokos GeorgeTsiokos force-pushed the fix/subscribe-async-silent-failure-1663 branch from 74b1b32 to 9ce96c6 Compare March 13, 2026 10:22
@WhitWaldo
Copy link
Contributor

Do note that I just merged a PR to support xUnit v3 so your unit/integration tests may need a refresh.

@GeorgeTsiokos GeorgeTsiokos marked this pull request as draft March 15, 2026 18:51
@GeorgeTsiokos GeorgeTsiokos force-pushed the fix/subscribe-async-silent-failure-1663 branch from 9ce96c6 to 4aba9a6 Compare March 15, 2026 20:23
- Throw DaprException wrapping RpcException on initial connection failure
- Add SubscriptionErrorHandler callback for runtime errors in background tasks
- Reset hasInitialized on any failure (initial or mid-stream) to allow retry
- Guard ErrorHandler invocation with try/catch to prevent unobserved exceptions
- Use CancellationToken.None for ContinueWith to ensure error handlers fire
- Include topic/pubsub names in error messages for multi-subscription debugging

Signed-off-by: George Tsiokos <george@tsiokos.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@GeorgeTsiokos GeorgeTsiokos force-pushed the fix/subscribe-async-silent-failure-1663 branch from 4aba9a6 to 5da307f Compare March 15, 2026 20:30
@GeorgeTsiokos GeorgeTsiokos marked this pull request as ready for review March 15, 2026 20:34
- Re-throw in HandleTaskCompletion when no ErrorHandler is set (preserves
  pre-existing UnobservedTaskException behavior)
- Clear stale clientStream on background failure to prevent retry reusing
  a dead stream
- Skip ErrorHandler invocation for OperationCanceledException (user
  cancellation is not an error)
- Fix inaccurate XML doc comments

Signed-off-by: George Tsiokos <george@tsiokos.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@GeorgeTsiokos

This comment was marked as outdated.

@GeorgeTsiokos
Copy link
Author

GeorgeTsiokos commented Mar 15, 2026

Out of scope for this PR:

  1. ProcessTopicChannelMessagesAsync swallows OperationCanceledException from the message handler — timeout-triggered cancellations are silently converted to the default response action with no logging.
  2. FetchDataFromSidecarAsync has a bare catch (Exception) (line 351) that silently swallows channel-write failures, which could mask backpressure or disposal races.
  3. No ILogger anywhere in PublishSubscribeReceiver — all the silent catches above exist because there's no logger to report to. Adding an ILogger parameter would let these be logged instead of swallowed.
  4. ContinueWith + TaskScheduler.Default — the continuation pattern is fragile; a failed error handler's exception becomes unobserved. Modern async/await with a top-level try/catch would be more robust.

- Change SubscriptionErrorHandler delegate from void to Task to support
  async error handling (logging sinks, alerting)
- Make HandleTaskCompletion async to await the error handler
- Update tests for async handler signature
- Fix PR comment to not call out issues in our own code as pre-existing

Signed-off-by: George Tsiokos <george@tsiokos.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Messaging] SubscribeAsync silently fails with unobserved task exceptions when Dapr sidecar is unavailable

2 participants